Expose Trino backend state via JMX #782
Merged
+106
−12
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Reviewer's GuideThis PR integrates JMX exposure for Trino backend cluster state by injecting an MBeanExporter into BackendStateManager, defining a ClusterStatsJMX bean with Managed annotations and exporting/updating it in updateStates, updates the DI configuration to bind BackendStateManager via Guice, and adds an integration test to verify the new JMX metrics are exposed properly. File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- Replace the fixed Thread.sleep in testClusterStatsJMX with a polling loop and timeout to avoid flakiness in the test.
- Consider using Map.computeIfAbsent for clusterStatsJMXs in updateStates to streamline the creation and registration logic.
- Add unregister logic for MBeans when a cluster is removed or its stats are no longer tracked to prevent JMX resource leaks.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Replace the fixed Thread.sleep in testClusterStatsJMX with a polling loop and timeout to avoid flakiness in the test.
- Consider using Map.computeIfAbsent for clusterStatsJMXs in updateStates to streamline the creation and registration logic.
- Add unregister logic for MBeans when a cluster is removed or its stats are no longer tracked to prevent JMX resource leaks.
## Individual Comments
### Comment 1
<location> `gateway-ha/src/main/java/io/trino/gateway/ha/router/BackendStateManager.java:68-71` </location>
<code_context>
+
+ public static class ClusterStatsJMX
+ {
+ int runningQueryCount;
+ int queuedQueryCount;
+ int numWorkerNodes;
+ TrinoStatus trinoStatus;
+
+ public ClusterStatsJMX(ClusterStats clusterStats)
</code_context>
<issue_to_address>
**suggestion:** Fields in ClusterStatsJMX should be private for encapsulation.
Making these fields private will prevent external classes from modifying them and improve encapsulation.
```suggestion
private int runningQueryCount;
private int queuedQueryCount;
private int numWorkerNodes;
private TrinoStatus trinoStatus;
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
gateway-ha/src/main/java/io/trino/gateway/ha/router/BackendStateManager.java
Outdated
Show resolved
Hide resolved
oneonestar
force-pushed
the
ypoon/backendstate_jmx
branch
from
8131821 to
54fe806
Compare
oneonestar
force-pushed
the
ypoon/backendstate_jmx
branch
from
54fe806 to
92b1151
Compare
ebyhr
approved these changes
Oct 10, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Expose Trino backend state via JMX
Additional context and related issues
JMX looks like this:
I didn't implement unregister logic, because under the current implementation
BackendStateManager.clusterStatswon't delete old Trino cluster. We may need another PR to refactorBackendStateManagerand*Observerclasses to support this.Release notes
(X) Release notes are required, with the following suggested text:
* Expose Trino cluster state via JMX.Summary by Sourcery
Expose Trino backend state via JMX by exporting dynamic ClusterStatsJMX MBeans for each cluster and updating dependency injection to support MBeanExporter
New Features:
Enhancements:
Tests: